[datafusion-spark] Add Spark-compatible ceil function#20593
[datafusion-spark] Add Spark-compatible ceil function#20593shivbhatia10 wants to merge 9 commits intoapache:mainfrom
Conversation
comphead
left a comment
There was a problem hiding this comment.
Thanks @shivbhatia10 it would be nice having Spark .slt tests covering edge cases and datatypes as well as commenting in file body what is the difference between Spark and current DF implementation
|
Hey @comphead, I've added some |
|
Thanks @shivbhatia10 would you mind providing an example where DF ceil and Spark ceil return diff result? I'm still struggling to understand the difference between them |
| /// - Spark's ceil on Decimal128(p, s) returns Decimal128(p−s+1, 0), reducing scale | ||
| /// to 0; DataFusion preserves the original precision and scale | ||
| /// - Spark only supports Decimal128; DataFusion also supports Decimal32/64/256 | ||
| /// - Spark does not check for decimal overflow; DataFusion errors on overflow |
There was a problem hiding this comment.
Hey @comphead , I've documented the differences here between the Spark and DataFusion ceil functions
There was a problem hiding this comment.
Yes, I was going through this, and this actually causing a question to find a query using ceil that currently behaves differently in DF and in Spark? it can be Spark Ansi mode as well
Which issue does this PR close?
Part of #15914 and apache/datafusion-comet#1704
I also just noticed #15916 butI believe work has been stale on this one.
Rationale for this change
Helping to continue adding Spark compatible expressions to datafusion-spark.
What changes are included in this PR?
Add new
ceilfunction.Are these changes tested?
Yes, unit tests.
Are there any user-facing changes?
No.